include metric name in annotations from histogram_quantile if delayed… #13905

tcp13equals2 · 2025-12-31T06:50:50Z

What this PR does

Now that MQE supports delayed name removal, it is possible to include the metric name in histogram_quantile() warning and info annotations.

This brings parity to the Prometheus implementation.

The metric names are only included in the relevant warning/info annotations if EnableDelayedNameRemoval is enabled.

Which issue(s) this PR fixes or relates to

Fixes #3299

Checklist

[x ] Tests updated.
Documentation added.
[ x] CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]. If changelog entry is not needed, please add the changelog-not-needed label to the PR.
about-versioning.md updated with experimental features.

Note

Adds metric names to histogram_quantile warning/info annotations when EnableDelayedNameRemoval is on, aligning with Prometheus behavior.

Plumbs delayed name removal into histogram functions; annotations now emit metric name conditionally
Refactors tests to run against engines with delayed name removal both enabled/disabled, with dual expected annotations
Adds ours/native_histograms_delayed_name_removal_disabled.test and enables related upstream tests; updates optimizer test skips accordingly
Updates CHANGELOG.md with enhancement entry

^{Written by Cursor Bugbot for commit fe279ba. This will update automatically on new commits. Configure here.}

… name removal is enabled

pkg/streamingpromql/operators/functions/histogram_function.go

fionaliao

LGTM

charleskorn · 2026-01-05T00:16:47Z

pkg/streamingpromql/testdata/upstream/native_histograms.test

Why have these test cases been disabled? Weren't they previously passing?

The TestEliminateDeduplicateAndMergeOptimizationWithDelayedNameRemovalDisabled() test case fails on this upstream file because it runs with delated name removal disabled.

Rather then disable the tests I add another Skip inside this test for the specific file. Note that the tests have been copied into the 2 new "ours" files, so we still get the same test coverage.

So ours/native_histograms_delayed_name_removal_enabled.test should be identical to upstream/native_histograms.test? If so, do we need the copy in ours?

I only copied out the tests relevant to the new files. So the file is not identical.

Since we will uncomment the Unsupported by streaming engine from the upstream file, I'll remove the native_histograms_delayed_name_removal_enabled.test as this will not be needed.

We will just keep the _disabled.test file.

pkg/streamingpromql/engine_test.go

pkg/streamingpromql/operators/functions/histogram_function.go

tacole02

Changelog LGTM

…ogram_quantile

CHANGELOG.md

charleskorn · 2026-01-19T23:02:38Z

pkg/streamingpromql/engine_test.go

 			data:                    mixedClassicHistograms,
 			expr:                    `histogram_quantile(0.5, series{host="d"})`,
 			expectedInfoAnnotations: []string{`PromQL info: input to histogram_quantile needed to be fixed for monotonicity (see https://prometheus.io/docs/prometheus/latest/querying/functions/#histogram_quantile) (1:25)`},
+			expectedInfoAnnotationsDelayedNameRemovalEnabled: []string{`PromQL info: input to histogram_quantile needed to be fixed for monotonicity (see https://prometheus.io/docs/prometheus/latest/querying/functions/#histogram_quantile) for metric name "series" (1:25)`},


I realise this isn't something we can control, but I find the expression here hard to follow - the "for metric name ..." bit seems out of place after the parentheses, it fits better before the parentheses to me.

Perhaps something to change upstream?

Which ones do you like;

The histogram_quantile input for metric "series" was corrected to enforce monotonicity (1:25) - see ....

The histogram_quantile input for metric "series" was fixed to enforce monotonicity (1:25) - see ....

Corrected non-monotonic histogram_quantile input for metric "series" (1:25) - see ....

Fixed non-monotonic histogram_quantile input for metric "series" (1:25) - see ....

I'm OK with tweaking the old structure:

input to histogram_quantile needed to be fixed for monotonicity for metric name "series" (see https://prometheus.io/docs/prometheus/latest/querying/functions/#histogram_quantile)

pkg/streamingpromql/optimize/plan/eliminate_deduplicate_and_merge_test.go

charleskorn · 2026-01-19T23:08:35Z

pkg/streamingpromql/testdata/upstream/native_histograms.test

So ours/native_histograms_delayed_name_removal_enabled.test should be identical to upstream/native_histograms.test? If so, do we need the copy in ours?

pkg/streamingpromql/engine_test.go

tcp13equals2 · 2026-01-19T23:32:15Z

@charleskorn Thanks for your feedback and suggestions. Do you have any other feedback for this PR?

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is ON, but it could not run because the Cursor GitHub App does not have access to this repository. If the app is installed on your organization with "selected repositories", make sure this repository is included in the selection.}

pkg/streamingpromql/engine_test.go

pkg/streamingpromql/optimize/plan/eliminate_deduplicate_and_merge_test.go

pkg/streamingpromql/testdata/ours/native_histograms_delayed_name_removal_enabled.test

charleskorn · 2026-01-20T00:06:09Z

pkg/streamingpromql/engine_test.go

 			data:                    mixedClassicHistograms,
 			expr:                    `histogram_quantile(0.5, series{host="d"})`,
 			expectedInfoAnnotations: []string{`PromQL info: input to histogram_quantile needed to be fixed for monotonicity (see https://prometheus.io/docs/prometheus/latest/querying/functions/#histogram_quantile) (1:25)`},
+			expectedInfoAnnotationsDelayedNameRemovalEnabled: []string{`PromQL info: input to histogram_quantile needed to be fixed for monotonicity (see https://prometheus.io/docs/prometheus/latest/querying/functions/#histogram_quantile) for metric name "series" (1:25)`},


I'm OK with tweaking the old structure:

input to histogram_quantile needed to be fixed for monotonicity for metric name "series" (see https://prometheus.io/docs/prometheus/latest/querying/functions/#histogram_quantile)

tcp13equals2 · 2026-01-20T00:14:44Z

@charleskorn The un-necessary test file has been removed and the test string updated. Anything else you can see needs addressing?

charleskorn

LGTM modulo last remaining reference to deleted test file:

pkg/streamingpromql/optimize/plan/eliminate_deduplicate_and_merge_test.go

tcp13equals2 added 4 commits December 31, 2025 14:50

include metric name in annotations from histogram_quantile if delayed…

e92c5fc

… name removal is enabled

Update CHANGELOG.md

f941eb3

Separate test files

c3abc4d

Tweaks

2d76138

tcp13equals2 marked this pull request as ready for review January 2, 2026 00:15

tcp13equals2 requested a review from a team as a code owner January 2, 2026 00:15

cursor bot reviewed Jan 2, 2026

View reviewed changes

pkg/streamingpromql/operators/functions/histogram_function.go Show resolved Hide resolved

tcp13equals2 added 2 commits January 2, 2026 08:29

Update histogram_function.go

2d6fe0a

More differences

b0a10e2

cursor bot reviewed Jan 2, 2026

View reviewed changes

pkg/streamingpromql/operators/functions/histogram_function.go Show resolved Hide resolved

fionaliao approved these changes Jan 2, 2026

View reviewed changes

charleskorn reviewed Jan 5, 2026

View reviewed changes

tacole02 reviewed Jan 5, 2026

View reviewed changes

tcp13equals2 added 4 commits January 19, 2026 11:31

Update native_histograms.test

22cad81

Update engine_test.go

eb62803

Merge branch 'main' into include-metric-name-in-annotations-from-hist…

cb68b15

…ogram_quantile

Update eliminate_deduplicate_and_merge_test.go

73819a8

charleskorn reviewed Jan 19, 2026

View reviewed changes

PR feedback

71b15b9

cursor bot reviewed Jan 19, 2026

View reviewed changes

pkg/streamingpromql/engine_test.go Show resolved Hide resolved

charleskorn reviewed Jan 20, 2026

View reviewed changes

PR feedback

3bbefd1

charleskorn approved these changes Jan 20, 2026

View reviewed changes

pkg/streamingpromql/optimize/plan/eliminate_deduplicate_and_merge_test.go Outdated Show resolved Hide resolved

Update eliminate_deduplicate_and_merge_test.go

fe279ba

tcp13equals2 merged commit bfd7400 into main Jan 20, 2026
39 checks passed

tcp13equals2 deleted the include-metric-name-in-annotations-from-histogram_quantile branch January 20, 2026 00:53

include metric name in annotations from histogram_quantile if delayed… #13905

include metric name in annotations from histogram_quantile if delayed… #13905

Uh oh!

Conversation

tcp13equals2 commented Dec 31, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Which issue(s) this PR fixes or relates to

Checklist

Uh oh!

Uh oh!

Uh oh!

fionaliao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tacole02 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tcp13equals2 commented Jan 19, 2026

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tcp13equals2 commented Jan 20, 2026

Uh oh!

charleskorn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tcp13equals2 commented Dec 31, 2025 •

edited by cursor bot

Loading